Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a "(no title)" label to links to pages or posts with no title #19528

Merged
merged 3 commits into from
Jan 13, 2020

Conversation

draganescu
Copy link
Contributor

Description

Closes: #18897

How has this been tested?

Tested locally by:

  • created a page with no title
  • in a post inserted a navigation block
  • chose create from existing pages

Screenshots

no-title

Types of changes

Non breaking change to how default links are created in a navigation block.

@WunderBart
Copy link
Member

Do you think we could wait with this one until we're able to add the post ID as a prefix there? (#18897 (comment))

@obenland
Copy link
Member

obenland commented Jan 9, 2020

I'm not sure post ids should necessarily be user facing like that? In the comment you linked to you said

as it's currently done in the Primary Menu

Can you link me to the code that does that?

@draganescu
Copy link
Contributor Author

@WunderBart why should we wait on merging this? Seems like a minor non-breaking change to me.

@WunderBart
Copy link
Member

WunderBart commented Jan 13, 2020

Can you link me to the code that does that?

@obenland, I believe the code is here (there are more instances, search for #%d (no title)). On the page it looks like this (TwentyTwenty):

image


why should we wait on merging this? Seems like a minor non-breaking change to me.

@draganescu, I thought it was worth a wait because of 2 things:

  1. The primary nav does it (as mentioned above), so to keep consistency.
  2. To avoid cases where we'd have indistinguishable (no title) (no title) (no title) nav (it's an edge-case but an easy one to avoid).

I'm not considering it a blocker, just a suggestion. We can always add it in another PR!

@draganescu
Copy link
Contributor Author

draganescu commented Jan 13, 2020

Ah I see @WunderBart and I think you do have a point about that edge case. Nevertheless, considering that now the users get a "buggy" no-label-whatsoever menu item, I will merge this and choose, as you also suggested, to come back in a second PR to add the ID for consistency and edge cases.

Thank you! :)

@draganescu draganescu merged commit e7342de into master Jan 13, 2020
@draganescu draganescu deleted the fix/no-title-pages-in-menu branch January 13, 2020 14:47
@ellatrix ellatrix added this to the Gutenberg 7.3 milestone Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation Block adds an empty link for untitled pages
4 participants